refactor(rollout): engine-level sync/async contracts for async rollout - #4
Closed
celve wants to merge 3 commits into
Closed
refactor(rollout): engine-level sync/async contracts for async rollout#4celve wants to merge 3 commits into
celve wants to merge 3 commits into
Conversation
Handle.launch_nowait runs the dispatch -> localize -> execute half of a @distributed method call and returns a PendingHandleCall future (ready/wait/result) instead of blocking in ray.get; result() runs the rebind + collect half at most once (cached on success, retryable on failure). _bind_methods records each method's dispatch tuple so the seam works for any @distributed method. handle_fn itself is untouched; the async rollout path builds on this instead of reaching into the private _execute_all/_rebind_tree seams.
… contracts Replace the layered async_runtime (AsyncRolloutScheduler + GenerationDispatcher protocol + RayGenerationDispatcher + VersionedGroupBuffer) and the agentic trainers' duplicated producer plumbing (_GroupAssembler/_GroupBuffer/ _ingest_completed/_pump/_drain_buffer) with two driver-side engines built on two mechanism classes, all under unirl/rollout/engine/: - synchronous.py (was base.py): the worker-side sync contracts, unchanged in substance; BaseSingleTurnRolloutEngine is renamed SyncRolloutEngine and chunked_engine_generate moves in from the package __init__. - asynchronous.py (new): VersionedBuffer + InflightPool (ray-free mechanisms over Handle.launch_nowait), the AsyncRolloutEngine protocol (poll / drain_freshest / pop_evicted / quiesce + engine-owned weight_version), and its two concretes: AsyncBatchRolloutEngine (launch-time version stamps; used by AsyncARTrainer/AsyncDiffusionTrainer) and AsyncAgenticRolloutEngine (facade over the agentic rank-0 coordinator: [0] unwraps, PendingGroups sibling assembly, completion-time stamps, quiesce = abort + final poll; used by the partial/async agentic trainers). - engine/__init__.py becomes a lazy re-export shim so the driver-side module imports ray/torch-free. Trainers keep the policy (launch ceiling, reap-vs-launch order as statement order, quiesce points, tail carry/drop) as explicit loops; the reap_before_launch flag, dead drained()/pending_roots()/_pending_carried, and the trainer-side _weight_version counters are removed. tests/ gains CPU-only coverage for the buffer/pool invariants (complete-or-nothing retry, freshest-first eviction, quiesce-before-bump stamping).
Owner
Author
|
Superseded by the upstream PR: Tencent-Hunyuan#287 (this one was opened against the fork's main by mistake). |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Unifies the two async rollout paths behind engine-level contracts under
unirl/rollout/engine/, replacing the over-layeredasync_runtime.py(scheduler + one-implementation dispatcher Protocol + callback types) and the agentic trainers' duplicated producer plumbing.engine/synchronous.py(wasbase.py): worker-side sync contracts.BaseSingleTurnRolloutEngineis renamedSyncRolloutEngine;chunked_engine_generatemoves in from the package__init__.engine/asynchronous.py(new): the driver side —VersionedBuffer+InflightPoolmechanisms, theAsyncRolloutEngineprotocol (poll/drain_freshest/pop_evicted/quiesce, engine-ownedweight_version), and two concretes:AsyncBatchRolloutEngine(AR/diffusion; launch-time version stamps) andAsyncAgenticRolloutEngine(facade over the agentic rank-0 coordinator;[0]unwraps,PendingGroupssibling assembly, completion-time stamps,quiescefolds abort + the mandatory follow-up poll).Handlegains a public non-blocking seam (launch_nowait→PendingHandleCall.ready()/wait()/result()), removingRayGenerationDispatcher's documented "update in lockstep" coupling to the private_execute_all/_rebind_tree;handle_fnis untouched.150s→8s BAGEL constraint; quiesce before sync; tail carry/drop). Thereap_before_launchflag, deaddrained()/pending_roots()/_pending_carried, and trainer-side_weight_versioncounters are removed.engine/__init__.pybecomes a lazy re-export shim so the driver-side module imports ray/torch-free, enabling the new CPU-only unit tests.Net −13 lines with tests added; the runtime layer itself shrinks ~390 → ~450 lines covering both paths instead of one.
Related Issue
N/A (tracked in Linear as LIN-631).
Test Plan
ruff check unirl/rollout unirl/trainer unirl/distributed/group/handle.py tests— passes.python3 -m compileall -q unirl/rollout unirl/trainer unirl/distributed/group/handle.py tests— passes.uvx pytest tests/ -q— 24 passed (new CPU-only tests pinning: complete-or-nothing reap retry without double-insert, first-error deferral, KeyboardInterrupt passthrough, freshest-first staleness eviction, launch-time vs completion-time version stamping, quiesce-poll-before-bump ordering, n-sibling group assembly).examples/ar/qwen3_grpo_4b_base_dapo_sglang_async.yaml, the BAGELbagel_vllmomni_async.yaml, and an ALFWorld partial/async recipe — recommended before merge; the diffusion reap-ordering and launch-ceiling invariants are only fully observable there.Compatibility / Risk
_target_paths and knobs are unchanged (reap_before_launchwas internal, never a recipe key).unirl.rollout.async_runtime/ deleted names (AsyncRolloutScheduler,RayGenerationDispatcher,VersionedGroupBuffer,GenerationDispatcher,InflightGeneration) are gone with no aliases;unirl.rollout.engine.base→unirl.rollout.engine.synchronous. All in-repo consumers are migrated; out-of-tree code importing these must update._next_steploops (reviewed line-by-line against the deletednext_stepfor both orderings) and the agenticquiesce()fold (test-pinned: groups completing during the quiesce carry the pre-sync version)._pending_carriedremoval is a verified dead-state deletion (only ever assigned[]).Reviewer Notes
handle.pyseam (commit 1, additive) →engine/asynchronous.pymechanisms → protocol + engines →async_ar/async_diffusionloops (check reap ordering) →agentic_partial/agentic_async(check quiesce-before-bump and tail-policy counter divergence) → tests as the executable spec._next_step/_build_tasksloop pairs, makinghandle_fndelegate to the seam halves, and the_connect_separate/two-slab-build duplication.Checklist